Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add psalm template annotation to container get method #30498

Closed
wants to merge 2 commits into from

Conversation

juliusknorr
Copy link
Member

Add psalm annotations in order to properly detect the return type of \OC::$server->get(MyClass::class).

Even though DI should be used, this allows to properly get the return type without manually specifiying it when using get()

@juliusknorr juliusknorr added enhancement 3. to review Waiting for reviews labels Jan 5, 2022
@juliusknorr juliusknorr added this to the Nextcloud 24 milestone Jan 5, 2022
@juliusknorr juliusknorr requested review from ChristophWurst, a team, nickvergessen, ArtificialOwl and CarlSchwan and removed request for a team January 5, 2022 12:30
@CarlSchwan
Copy link
Member

As I recently discovered, it seems like the Static code analysis / static-code-analysis-ocp and Static code analysis / static-code-analysis are not run correctly anymore and don't report errors anymore, so we should refrain to merge this until we figure out what is wrong (I'm on it)

@nickvergessen nickvergessen marked this pull request as draft January 7, 2022 09:18
@CarlSchwan CarlSchwan marked this pull request as ready for review January 14, 2022 09:20
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@CarlSchwan CarlSchwan force-pushed the docs/psalm-container branch 2 times, most recently from ac108c4 to 987c53b Compare January 14, 2022 10:10
@CarlSchwan
Copy link
Member

I added a commit that fixes the new issues reported by psalm. Please check it :)

@CarlSchwan CarlSchwan force-pushed the docs/psalm-container branch 3 times, most recently from bff799b to e184c31 Compare January 14, 2022 10:24
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@CarlSchwan CarlSchwan force-pushed the docs/psalm-container branch from e184c31 to 623865c Compare January 14, 2022 12:01
@@ -1974,6 +1984,7 @@ public function getWebRoot() {
* @deprecated 20.0.0
*/
public function getOcsClient() {
/** @psalm-suppress UndefinedClass */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw this code looks broken since I couldn't find where the class or alias OcsClient is defined. This, fortunately, doesn't seems to be called anywhere: http://carlschwan.eu:6070/search?q=OcsClient

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
@blizzz blizzz mentioned this pull request Apr 7, 2022
@blizzz blizzz mentioned this pull request Apr 13, 2022
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
This was referenced Aug 12, 2022
This was referenced Aug 24, 2022
This was referenced Sep 6, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@juliusknorr
Copy link
Member Author

Obsolete with #33388

@nickvergessen nickvergessen deleted the docs/psalm-container branch October 14, 2022 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants